feat(#11): include input checkbox for task lists #12
Conversation
|
Hi @KyleKing! Yes, emoji test were broken in the master as tests download fresh HTML from djot syntax spec page (https://htmlpreview.github.io/?https://github.com/jgm/djot/blob/master/doc/syntax.html) which recently changed Regarding the approach - what do you think if input checkbox will be supported only in the rendering layer? I see drawback in the current approach as new AST for checkboxed-list can make maintenance harder because on one hand - this is new AST node but on the other hand - it must behave just as regular list and the only difference is in the presentation layer. I would suggest to add this feature only in the rendering layer by extending logic of The development flow is simple and you did everything right: |
Can't use because requires popping the class Key
This reverts commit ab4b5a7.
There was a problem hiding this comment.
Ready for review again! Feel free to make changes directly if easier than providing feedback
I also ran fuzz testing locally (go test -fuzz FuzzDjotTokenizer -fuzz FuzzDjotE2E github.com/sivukhin/godjot/djot_parser). I'm not sure how long to let it run, but it ran for two minutes after this fix without finding any new issues.
fuzz: elapsed: 0s, gathering baseline coverage: 0/699 completed
fuzz: elapsed: 0s, gathering baseline coverage: 699/699 completed, now fuzzing with 10 workers
fuzz: elapsed: 3s, execs: 165661 (55208/sec), new interesting: 13 (total: 712)
fuzz: elapsed: 6s, execs: 338017 (57444/sec), new interesting: 26 (total: 725)
...
fuzz: elapsed: 2m6s, execs: 5540830 (33608/sec), new interesting: 191 (total: 890)
^Cfuzz: elapsed: 2m7s, execs: 5553930 (23612/sec), new interesting: 191 (total: 890)
PASS
ok github.com/sivukhin/godjot/djot_parser 127.064s
`go test -run=FuzzDjotE2E/326270ac6d2da709 github.com/sivukhin/godjot/djot_parser`
This reverts commit 4b28fbd.
There was a problem hiding this comment.
I tweaked djot markup in todo-list test a bit because there are nuances related to the EOF handling in godjot
I will merge this branch. @KyleKing - thanks one more time for contribution! Sorry that it took longer than it actually needed to be!
This is the first go project I've actually worked on, so any guidance would be appreciated! I've been running
make test lintlocally, but the Emoji test (35) fails and I think my changes are generally the right direction, but I'm not sure what to do to finish the changeFixes #11